Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace secp256k1 with @noble/secp256k1 #599

Merged
merged 11 commits into from
May 9, 2022
Merged

Replace secp256k1 with @noble/secp256k1 #599

merged 11 commits into from
May 9, 2022

Conversation

D4nte
Copy link
Contributor

@D4nte D4nte commented Mar 6, 2022 •
edited
Loading

Helps with #337:

  1. @noble/secp256k1 is already imported by libp2p:
▶ npm ls @noble/secp256k1
[email protected] /home/fryorcraken/src/status-im/js-waku
└─┬ [email protected]
  └─┬ [email protected]
    └── @noble/[email protected]
  1. @noble/secp256k1 uses subtle crypto in the browser:

https://github.com/paulmillr/noble-secp256k1/blob/97aa518b9c12563544ea87eba471b32ecf179916/index.ts#L4

  1. Hence, no more crypto-browserify.

@github-actions
Copy link

github-actions bot commented Mar 6, 2022 •
edited
Loading

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
build/esm/index.js 1.66 MB (-21.37% 🔽) 34.1 s (-21.37% 🔽) 1.6 s (-14.26% 🔽) 35.6 s

@D4nte
Copy link
Contributor Author

D4nte commented Mar 7, 2022

Looks like we need to do #579 first so that browser field is used.

@D4nte D4nte mentioned this pull request Mar 7, 2022
Open
@D4nte D4nte force-pushed the remove-node-crypto branch 2 times, most recently from f28fc28 to bcc0b64 Compare March 7, 2022 02:37
@D4nte D4nte changed the title Remove node crypto usage Remove secp256k1 dep Mar 7, 2022
@D4nte D4nte added the blocked This issue is blocked by some other work label Mar 14, 2022
@D4nte D4nte changed the title Remove secp256k1 dep Replace secp256k1 with @noble/secp256k1 May 9, 2022
@D4nte D4nte force-pushed the remove-node-crypto branch 3 times, most recently from 51da1e5 to a3f7f80 Compare May 9, 2022 07:58
@D4nte D4nte marked this pull request as ready for review May 9, 2022 13:33
@D4nte D4nte removed the blocked This issue is blocked by some other work label May 9, 2022
@D4nte D4nte merged commit 0e42cf6 into master May 9, 2022
@D4nte D4nte deleted the remove-node-crypto branch May 9, 2022 23:57
@D4nte D4nte mentioned this pull request May 19, 2022
Closed
@D4nte D4nte mentioned this pull request Jun 22, 2022
Closed
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@D4nte